Can we add a test to ensure that parallel processing of RPC requests work?

a8d4eb6

edit: demonstrates that multiple requests can be in flight simultaneously and that the context is isolated

If the processing was sequential the added test would still pass I think. Do we not at a minimum need an element of time difference between the requests? Ideally we can prove that requests don't always flow through in the order they are dispatched in

866aed6 demonstrates that request are not processed serially

Is there any way to avoid from needing to typecast context values? I see that we have assertGet, so I imagine that this would be the primary way to access values in the context instead of get, but since we aren't performing any validation on values it seems like a fancy typecast.

If the request and result are generic, should the context be generic too? Perhaps we could add another type parameter to JsonRpcMiddleware, after the request. Maybe it would work something like this:

const middleware1 = (({ context, next }) => {
  return context.get('foo') < 30 ? 'bar' : next();
}) satisfies JsonRpcMiddleware<
  JsonRpcRequest,
  // The second parameter is now the content of the context
  { foo: number },
  'bar' | true
>;

const middleware2 = (() => {
  return true;
}) satisfies JsonRpcMiddleware<JsonRpcRequest, {}, true>;

const engine = new JsonRpcEngineV2<
  JsonRpcRequest,
  // The second parameter is now the content of the context
  { foo: number },
  'bar' | true
>({
  middleware: [middleware1, middleware2],
});

const request = makeRequest();

const result1 = await engine.handle(request, {
  context: new MiddlewareContext({ foo: 31 }),
});  //=> 'bar'

const result2 = await engine.handle(request, {
  context: new MiddlewareContext({ foo: 29 }),
});  //=> true

To do this, I suppose MiddlewareContext would have to be a wrapper around an object rather than a subclass of Map.

Would this introduce safety issues and/or screw up compatibility between V1 and V2?

I think I have a solution for this, will give it a go.

[Thought] In providing default values for these parameters are we expecting engineers to be using this type directly? If so, I wonder if that is the most ergonomic approach. I wonder if we can almost always have TypeScript infer this. Maybe not something we have to improve now, but just a thought.

Inference at the time of engine construction generally works, although if you're defining a middleware function in one place for an engine that lives elsewhere, you probably want to use this type to ensure that your middleware function is valid. Do you have any ideas as to how we would make that more ergonomic?

I would be curious how far we could get with removing the default values. I was mainly thinking it would reduce the size of this file. But maybe it wouldn't, and perhaps you're right that in practice it wouldn't matter as developers would need to pass in these types anyway. Like I said, just a thought as I worked with this file a bit locally, not anything concrete to suggest here.

[Thought] I wonder if we should do another pass through these tests at some point and convert these type assertions into satisfies? I do agree with Jongsun that in general we should be promoting use of satisfies at least for non-final types instead of type assertions, so that we can have TypeScript infer as much as it can. I'd at least want the tests to serve as examples and represent best practices for how authors should define middleware.